[Bugfix] Fix IndexError in Qwen3CoderToolParser streaming#34495
[Bugfix] Fix IndexError in Qwen3CoderToolParser streaming#34495R3hankhan123 wants to merge 1 commit into
Conversation
|
@haosdent fyi |
There was a problem hiding this comment.
Code Review
This pull request addresses an IndexError in the Qwen3CoderToolParser during streaming tool calls by adding necessary boundary checks before accessing self.streamed_args_for_tool. The fix appears correct in preventing the crash. However, the same checking logic has been duplicated in four separate locations within the extract_tool_calls_streaming method. My review includes a suggestion to refactor this duplicated code into a single helper method to improve the code's maintainability and reduce redundancy.
1b7a345 to
03bd302
Compare
|
Here is my local change, you may take a reference and update the PR if need diff --git a/tests/tool_parsers/test_qwen3coder_tool_parser.py b/tests/tool_parsers/test_qwen3coder_tool_parser.py
index 3d46f73de..b5611e930 100644
--- a/tests/tool_parsers/test_qwen3coder_tool_parser.py
+++ b/tests/tool_parsers/test_qwen3coder_tool_parser.py
@@ -40,7 +40,7 @@ def qwen3_xml_tool_parser(qwen3_tokenizer):
return Qwen3XMLToolParser(qwen3_tokenizer)
-@pytest.fixture(params=["xml"])
+@pytest.fixture(params=["original", "xml"])
def qwen3_tool_parser_parametrized(qwen3_tool_parser, qwen3_xml_tool_parser, request):
"""Parameterized fixture that provides both parser types for testing"""
if request.param == "original":
@@ -668,6 +668,17 @@ def test_extract_tool_calls_streaming(
expected_tool_calls
)
+ # Verify streamed_args_for_tool contract with serving layer:
+ # len must match prev_tool_call_arr, and each entry must be valid JSON.
+ # This prevents IndexError when the serving layer accesses
+ # streamed_args_for_tool[index] at generation finish (issue #34322).
+ parser = qwen3_tool_parser_parametrized
+ assert len(parser.streamed_args_for_tool) == len(parser.prev_tool_call_arr)
+ for i in range(len(parser.prev_tool_call_arr)):
+ streamed = parser.streamed_args_for_tool[i]
+ assert streamed, f"streamed_args_for_tool[{i}] should not be empty"
+ json.loads(streamed) # must be valid JSON
+
# Verify each tool call
for idx, expected_tool in enumerate(expected_tool_calls):
state = tool_states[idx]
@@ -899,12 +910,14 @@ def test_extract_tool_calls_complex_type_with_single_quote(
def test_extract_tool_calls_streaming_missing_opening_tag(
- qwen3_tool_parser_parametrized, qwen3_tokenizer, sample_tools
+ qwen3_xml_tool_parser, qwen3_tokenizer, sample_tools
):
"""Test streaming with missing opening <tool_call> tag
- This tests that the streaming parser correctly handles
- tool calls that start directly with <function=...>
+ This tests that the XML streaming parser correctly handles
+ tool calls that start directly with <function=...>.
+ This is an XML parser-specific fallback; the original parser
+ requires the <tool_call> wrapper.
"""
model_output = """I'll check the weather for you.
@@ -927,7 +940,7 @@ fahrenheit
tool_states = {}
for delta_message in stream_delta_message_generator(
- qwen3_tool_parser_parametrized, qwen3_tokenizer, model_output, request
+ qwen3_xml_tool_parser, qwen3_tokenizer, model_output, request
):
if delta_message.content:
other_content += delta_message.content
@@ -963,7 +976,7 @@ fahrenheit
# Verify we got the tool call
assert len(tool_states) == 1
- assert len(qwen3_tool_parser_parametrized.prev_tool_call_arr) == 1
+ assert len(qwen3_xml_tool_parser.prev_tool_call_arr) == 1
state = tool_states[0]
assert state["id"] is not None
diff --git a/vllm/tool_parsers/qwen3coder_tool_parser.py b/vllm/tool_parsers/qwen3coder_tool_parser.py
index a3c79f865..0b5590fb9 100644
--- a/vllm/tool_parsers/qwen3coder_tool_parser.py
+++ b/vllm/tool_parsers/qwen3coder_tool_parser.py
@@ -482,17 +482,13 @@ class Qwen3CoderToolParser(ToolParser):
# IMPORTANT: Add to prev_tool_call_arr immediately when
# we detect a tool call. This ensures
# finish_reason="tool_calls" even if parsing isn't complete
- already_added = any(
- tool.get("name") == self.current_function_name
- for tool in self.prev_tool_call_arr
+ self.prev_tool_call_arr.append(
+ {
+ "name": self.current_function_name,
+ "arguments": {}, # Placeholder, will be updated later
+ }
)
- if not already_added:
- self.prev_tool_call_arr.append(
- {
- "name": self.current_function_name,
- "arguments": "{}", # Placeholder, will be updated later
- }
- )
+ self.streamed_args_for_tool.append("")
# Send header with function info
return DeltaMessage(
@@ -514,6 +510,7 @@ class Qwen3CoderToolParser(ToolParser):
# Send opening brace if not sent yet
if not self.json_started and self.parameter_prefix not in delta_text:
self.json_started = True
+ self.streamed_args_for_tool[self.current_tool_index] += "{"
return DeltaMessage(
tool_calls=[
DeltaToolCall(
@@ -550,16 +547,14 @@ class Qwen3CoderToolParser(ToolParser):
else None,
)
if parsed_tool:
- # Update existing entry in
- # prev_tool_call_arr with complete args
- for i, tool in enumerate(self.prev_tool_call_arr):
- if tool.get("name") == parsed_tool.function.name:
- args = parsed_tool.function.arguments
- self.prev_tool_call_arr[i]["arguments"] = args
- break
+ # Update current tool entry with complete args
+ self.prev_tool_call_arr[self.current_tool_index][
+ "arguments"
+ ] = json.loads(parsed_tool.function.arguments)
except Exception:
pass # Ignore parsing errors during streaming
+ self.streamed_args_for_tool[self.current_tool_index] += "}"
result = DeltaMessage(
tool_calls=[
DeltaToolCall(
@@ -675,6 +670,9 @@ class Qwen3CoderToolParser(ToolParser):
)
self.param_count += 1
+ self.streamed_args_for_tool[self.current_tool_index] += (
+ json_fragment
+ )
return DeltaMessage(
tool_calls=[
Compared to your pull request, it includes:
|
|
This pull request has merge conflicts that must be resolved before it can be |
|
This pull request has merge conflicts that must be resolved before it can be |
03bd302 to
2aafe23
Compare
|
Hi @R3hankhan123, the pre-commit checks have failed. Please run: uv pip install pre-commit>=4.5.1
pre-commit install
pre-commit run --all-filesThen, commit the changes and push to your branch. For future commits, Tip Is
|
When streaming tool calls, the parser attempted to access `arguments[func_start:]` before validating that `func_start` is within bounds. This caused an IndexError when the function name appeared at the very end of the streamed chunk. Signed-off-by: Rehan Khan <Rehan.Khan7@ibm.com>
2aafe23 to
f0fe907
Compare
Purpose
When streaming tool calls, the parser attempted to access
arguments[func_start:]before validating thatfunc_startis within bounds. This caused an IndexError when the function name appeared at the very end of the streamed chunk.Fixes #34322
Test Plan
Test Result
Server logs
Curl request
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.